check: track a set of checked packs for partial checks#9909
Conversation
Replace the single last-pack-checked marker with a persisted set of checked pack ids, so a pack added between partial runs is verified regardless of how its content-sha256 name sorts.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9909 +/- ##
==========================================
+ Coverage 85.13% 85.34% +0.21%
==========================================
Files 93 93
Lines 15899 15932 +33
Branches 2428 2429 +1
==========================================
+ Hits 13535 13597 +62
+ Misses 1654 1631 -23
+ Partials 710 704 -6 ☔ View full report in Codecov by Harness. |
ThomasWaldmann
left a comment
ThomasWaldmann
left a comment
There was a problem hiding this comment.
Also: does it handle Ctrl-C correctly?
Split the vertical import and trim an irrelevant line from the checked-packs comment.
mr-raj12
force-pushed
the
fix-check-set-of-checked-packs
branch
from
b9214b6 to
0e68c7e
Compare
Yes, store.store() writes atomically and appends a sha256 that's verified on load, so an interrupt can't corrupt the set or mark a pack checked when it wasn't. The worst case you redo a last one minute of work, capped at 1 min now that checkpoints run every 60s. |
Also shrink the partial-check checkpoint window from 5 minutes to 1 minute so a Ctrl-C between checkpoints redoes at most a minute of verification work.
mr-raj12
force-pushed
the
fix-check-set-of-checked-packs
branch
from
0e68c7e to
996331b
Compare
|
Docs not updated — data-structures.rst:34 still documents last-pack-checked ("key of last pack checked as text"). It should describe checked-packs instead. |
|
Checkpoint cost grows with repo size (note, not a blocker). The old checkpoint wrote a ~70-byte marker; the new one rewrites the whole table (41 bytes/pack) every 5 minutes — ~4 MB per checkpoint at 100k packs. Fine in practice, just no longer O(1). Guess we should reduce checkpoint frequency: every 30 mins. |
2 participants
The repository check writes a marker holding the last pack id it verified, so a partial check (
--max-duration) resumes by skipping every pack up to that marker. This assumes the pack listing is sorted.Pack files are named by the sha256 of their content, so a pack added between two partial runs can sort anywhere. If its id sorts before the marker, the resumed check skips it and never verifies it.
This replaces the marker with a persisted set of checked pack ids, a
HashTableNTmapping pack id to(timestamp, result). A pack is skipped only when it appears in the set recorded as intact; a pack recorded as corrupt is re-verified rather than skipped. So a newly added pack gets checked whatever its name sorts to, and a pack that was corrupt in an earlier run is looked at again. The set is dropped once a cycle has checked every pack.The set is managed by a
PackTrackerclass. It persists viastore.store()(atomic write) with a sha256 appended over the serialized table; on load a blob whose sha256 does not match is discarded, so a rotted set re-checks everything instead of skipping an unverified pack. Ctrl-C is therefore safe: the on-disk set is never partially written and never records a pack that was not verified.The cycle is checkpointed every 30 minutes rather than every minute: the checkpoint now rewrites the whole table (~41 bytes/pack) instead of a fixed ~70-byte marker, so on large repos each save is no longer O(1). At 30-minute intervals that cost stays negligible while an interrupt still redoes at most 30 minutes of verification work.
The set is stored at
cache/checked-packs;docs/internals/data-structures.rstis updated to document it in place of the old marker.Closes #9897